Schema walker only descends through properties — silent miss for arrays / additionalProperties / oneOf.

if (!body || typeof body !== 'object' || !schema?.properties) return [];

Today this works because both blocked fields (executable, args) sit directly under properties of launchOptions. But the PR sets up an open extension point: "add httpReadOnly: true to any future sensitive field." That contract breaks silently when:

• args items become objects with a nested sensitive sub-field — walker doesn't descend through items:.
• A field lives under additionalProperties (e.g., dynamic keys) — walker doesn't descend.
• A field lives in a oneOf / anyOf branch — walker doesn't descend.

The failure mode is the worst kind: a future contributor adds httpReadOnly: true to a sensitive field, no one notices the walker doesn't reach it, and the field is silently settable over HTTP. The annotation looks correct, the test for that field is green (it's never reached), and the security hole is invisible until discovered.

Two options:

• Document the limit explicitly on the function ("only descends through properties — annotating fields under items / additionalProperties will be silently ignored") and add an assertion in test setup that walks configSchema for any httpReadOnly keys not reachable from the root, OR
• Extend the walker to descend into items/additionalProperties (less code than you'd think).

Given this is a security primitive, option 2 is the safer long-term play.

httpReadOnly is a non-standard JSON Schema keyword — compatibility check.

Question: what schema validator does Percy use to validate /percy/config bodies (and the static config file)?

• AJV with strict: true would reject unknown keywords with an error.
• AJV with strict: false (or default in older versions) silently ignores them — fine for stripping, but the keyword becomes invisible to any tooling that reads the schema.
• Other tools that consume configSchema (docs generation, TypeScript type generation, IDE auto-completion, schema-aware validators) won't know what httpReadOnly means.

Worth either:

• Confirming the validator is lenient and adding a comment here noting the keyword is read by findHttpReadOnlyPaths in api.js (so a reader doesn't grep for it in vain), OR
• Defining httpReadOnly as a documented custom keyword via the validator's extension API (AJV's addKeyword) so it's first-class.

Also: customer-facing docs for POST /percy/config should list which fields are HTTP-read-only. Today a customer programmatically configuring executable or args gets silent loss with only a server-side WARN log they probably won't see.

Silent stripping — response shape doesn't surface what was rejected.

let body = req.body && stripBlockedConfigFields(req.body, logger('core:server'));
return res.json(200, {
  config: body ? percy.set(body) : percy.config,
  success: true
});

A caller (Percy SDK, custom HTTP client, internal tooling) sends { discovery: { launchOptions: { executable: '/foo' } } }, gets 200 OK with success: true and a config object that silently lacks their executable setting. The WARN is logged server-side but the caller never sees it. "Successfully ignored your security-sensitive setting" is not great DX.

Question: should the response include ignored_fields: ['discovery.launchOptions.executable'] so the caller can show an actionable message in their UI / log? Or even return 400 Bad Request for HTTP-only sensitive fields, matching how other validators behave?

Argument for silent: less info-leak to a hostile caller; existing pattern matches "strip unknown fields." Argument against: this isn't a malicious-caller scenario by design — /percy/config is localhost-only, the caller is presumably trusted. Surface info wins.

Also minor: logger('core:server') is invoked on every config request. If logger() does any non-trivial lookup or instance allocation, hoist it to module scope:

const configLog = logger('core:server');
// ...
let body = req.body && stripBlockedConfigFields(req.body, configLog);

structuredClone is faster + handles more types if you're on Node 17+.

let stripped = JSON.parse(JSON.stringify(body));

For JSON-typed HTTP bodies this works fine — nothing in the body should be a Date/Map/Set/Function. So pure nitpick. But:

• structuredClone(body) is ~3× faster on Node 17+ (V8 native).
• It preserves types JSON loses (Dates, Maps, undefined slots in arrays), which makes the helper safer if it ever gets reused beyond HTTP request bodies.
• It throws on un-cloneable values (functions, DOM nodes) rather than silently dropping them.

Given the helper is exported and prefixed _ (i.e., "private but usable"), the wider type-correctness matters more than the perf delta. Worth a swap unless there's a stated Node-version floor below 17 that I'm missing.